OrderBook: Add market prices using Oracles #1674

lemunozm · 2024-01-05T10:17:26Z

Description

Improve pallet-order-book to swap currencies with a market price.

Change list

For a full list of the breaking changes, see: https://kflabs.slack.com/archives/C04GARZ532T/p1706864698973649

lemunozm · 2024-01-05T10:17:40Z

Compiling with cargo check -p pallet-order-book

pallets/order-book/src/lib.rs

lemunozm

I think I already have the base logic. Looking for your (fast) feedback before getting more into details @mustermeiszer @wischli

libs/traits/src/lib.rs

pallets/order-book/src/lib.rs

wischli

A few comments. Did not see anything I disagree with, great work!

libs/traits/src/lib.rs

pallets/order-book/src/lib.rs

mustermeiszer

Need to look at more depth tomorrrow, but what I can already say is that

We need an additional extrinsic that is callable by T::AdminOrigin and allows to set a T::FeederId

libs/traits/src/lib.rs

pallets/order-book/src/lib.rs

lemunozm · 2024-01-10T10:04:12Z

Note: in case we need again the default inverse ratios in the future here is the reverted commit: 451a6c7

mustermeiszer

Looks good! Thanks!

pallets/order-book/src/lib.rs

lemunozm · 2024-02-01T13:12:35Z

I made a couple of modifications to the internal FI structure:

I've added a type Correlation to keep track of two symmetrical variables that are being correlated, forcing that they always are correlated.
I've split post_increase/decrease_swap() methods into two variants, one when the swap is actually a swap and another when the swap is actually a cancelation of another swap. Before that, there were a lot of concept mixed and with this explicit separation, I think the code is more easy to follow.

pallets/foreign-investments/src/entities.rs

mustermeiszer

Review Part 1: Order book

pallets/foreign-investments/src/entities.rs

mustermeiszer · 2024-02-02T09:59:10Z

pallets/order-book/src/lib.rs

-			// update if the pair exists.
-			let _old_min_order = TradingPair::<T>::get(&asset_in, &asset_out)?;
-			TradingPair::<T>::insert(&asset_in, &asset_out, min_order);
+			MarketFeederId::<T>::put(feeder_id);


An Event would be nice.

Given that FeederId is just a value in a ValueStorage, who would want to look for the event instead of reading the storage directly?

Events are a means of saying that the state changed, so yes indexers will most likely only see that change if there was an evenrt.

Ok, because the indexer can not also index "calls" done, right? Because in that case, just seeing that the call has been performed, they now the state has changed

Done anyways here: 4a0f52a

pallets/order-book/src/lib.rs

pallets/foreign-investments/src/swaps.rs

* minor renaming * fix: add same currency orderbook short cut * chore: enable test debugging for info * fix: FI integration tests * chore: enable debug logging * tests: fix after rebasing * chore: remove debug lines * chore: proof order of operators is not the issue * fix: swapped decrease amount * fix: ForeignIdToSwapId removal * tests: apply fixed ForeignIdToSwapId removal --------- Co-authored-by: lemunozm <[email protected]>

pallets/foreign-investments/src/entities.rs

wischli

LGTM! Thanks for all the sweat and effort you have put into this!

mustermeiszer

That is a huge one!! Thanks to both @lemunozm and @wischli for the effort and brain juice that went into this here!

lemunozm · 2024-02-02T17:06:00Z

Thanks both for your awesome feedback and support 🙌🏻

lemunozm requested review from mikiquantum and wischli January 5, 2024 10:17

lemunozm self-assigned this Jan 5, 2024

lemunozm requested review from mustermeiszer and removed request for mikiquantum January 5, 2024 10:18

lemunozm commented Jan 5, 2024

View reviewed changes

pallets/order-book/src/lib.rs Outdated Show resolved Hide resolved

lemunozm commented Jan 5, 2024

View reviewed changes

pallets/order-book/src/lib.rs Outdated Show resolved Hide resolved

lemunozm commented Jan 5, 2024

View reviewed changes

pallets/order-book/src/lib.rs Outdated Show resolved Hide resolved

lemunozm force-pushed the order-book/market-prices branch from 2e32be4 to 6b59210 Compare January 5, 2024 15:25

lemunozm commented Jan 5, 2024

View reviewed changes

libs/traits/src/lib.rs Outdated Show resolved Hide resolved

pallets/order-book/src/lib.rs Outdated Show resolved Hide resolved

pallets/order-book/src/lib.rs Outdated Show resolved Hide resolved

lemunozm commented Jan 5, 2024

View reviewed changes

pallets/order-book/src/lib.rs Outdated Show resolved Hide resolved

lemunozm commented Jan 5, 2024

View reviewed changes

pallets/order-book/src/lib.rs Outdated Show resolved Hide resolved

lemunozm changed the title ~~OrderBook: Add market prices~~ OrderBook: Add market prices using Oracles Jan 5, 2024

lemunozm commented Jan 6, 2024

View reviewed changes

pallets/order-book/src/lib.rs Outdated Show resolved Hide resolved

wischli reviewed Jan 8, 2024

View reviewed changes

wischli added D5-breaks-api Pull request changes public api. Document changes publicly. I8-enhancement An additional feature. labels Jan 8, 2024

wischli added this to the Centrifuge 1025 milestone Jan 8, 2024

lemunozm commented Jan 8, 2024

View reviewed changes

pallets/order-book/src/lib.rs Outdated Show resolved Hide resolved

mustermeiszer reviewed Jan 8, 2024

View reviewed changes

libs/traits/src/lib.rs Outdated Show resolved Hide resolved

libs/traits/src/lib.rs Outdated Show resolved Hide resolved

mustermeiszer reviewed Jan 9, 2024

View reviewed changes

pallets/order-book/src/lib.rs Outdated Show resolved Hide resolved

pallets/order-book/src/lib.rs Outdated Show resolved Hide resolved

pallets/order-book/src/lib.rs Outdated Show resolved Hide resolved

pallets/order-book/src/lib.rs Outdated Show resolved Hide resolved

lemunozm force-pushed the order-book/market-prices branch from ae20e66 to 451a6c7 Compare January 10, 2024 10:05

lemunozm mentioned this pull request Jan 10, 2024

FI: Support for order book with market prices #1687

Merged

mustermeiszer previously approved these changes Jan 10, 2024

View reviewed changes

lemunozm dismissed mustermeiszer’s stale review via 8f5e07b January 10, 2024 19:13

wischli reviewed Jan 11, 2024

View reviewed changes

pallets/order-book/src/lib.rs Outdated Show resolved Hide resolved

lemunozm added D8-migration Pull request touches storage and needs migration code. Q7-very-hard Can be done by an expert coder with a deep knowledge of the codebase. labels Jan 12, 2024

lemunozm added 3 commits February 1, 2024 13:39

add minor swap utility

15678c9

fix clippy

26409c4

Remove BaseInfo and collect from investment

5c3fc02

lemunozm added 2 commits February 1, 2024 15:45

make pre_decrease_swap more readable

f899f3b

improve correlation docs

e2e3091

lemunozm force-pushed the order-book/market-prices branch from 9e32a92 to e2e3091 Compare February 1, 2024 14:50

wischli reviewed Feb 1, 2024

View reviewed changes

pallets/foreign-investments/src/entities.rs Outdated Show resolved Hide resolved

wischli mentioned this pull request Feb 1, 2024

fix: integration tests for OrderBook market prices #1717

Merged

5 tasks

fix Decrease when increment issue

1665c3b

mustermeiszer reviewed Feb 2, 2024

View reviewed changes

lemunozm added 3 commits February 2, 2024 11:51

improve swap test to check swap id storages

5536efe

remove unused code

9d57e59

move TooMuchDecrease to its correct place

8f2ba86

mustermeiszer reviewed Feb 2, 2024

View reviewed changes

pallets/foreign-investments/src/swaps.rs Show resolved Hide resolved

pallets/foreign-investments/src/swaps.rs Show resolved Hide resolved

wischli mentioned this pull request Feb 2, 2024

Feat: Pallet-Token-Mux #1713

Merged

11 tasks

wischli added 2 commits February 2, 2024 13:41

Merge branch 'main' into order-book/market-prices

5b597d7

tests: use updated outbound msg sender, not treasury

217243e

mustermeiszer reviewed Feb 2, 2024

View reviewed changes

pallets/foreign-investments/src/entities.rs Show resolved Hide resolved

lemunozm added 3 commits February 2, 2024 16:26

Merge remote-tracking branch 'origin/main' into order-book/market-prices

9aba062

add Feeder event

4a0f52a

remove currency pair storage

e579147

wischli approved these changes Feb 2, 2024

View reviewed changes

mustermeiszer approved these changes Feb 2, 2024

View reviewed changes

lemunozm merged commit 8cd9197 into main Feb 2, 2024
9 checks passed

wischli deleted the order-book/market-prices branch February 5, 2024 08:37

annamehr mentioned this pull request Feb 5, 2024

Swaps page: Fulfill orders with a market price centrifuge/apps#1865

Open

wischli mentioned this pull request Feb 6, 2024

Feat: Non-identical foreign investment swap rates #1654

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OrderBook: Add market prices using Oracles #1674

OrderBook: Add market prices using Oracles #1674

lemunozm commented Jan 5, 2024 •

edited

Loading

lemunozm commented Jan 5, 2024

lemunozm left a comment

wischli left a comment

mustermeiszer left a comment

lemunozm commented Jan 10, 2024 •

edited

Loading

mustermeiszer left a comment

lemunozm commented Feb 1, 2024 •

edited

Loading

mustermeiszer left a comment

mustermeiszer Feb 2, 2024

lemunozm Feb 2, 2024

mustermeiszer Feb 2, 2024

lemunozm Feb 2, 2024

lemunozm Feb 2, 2024

wischli left a comment

mustermeiszer left a comment

lemunozm commented Feb 2, 2024

OrderBook: Add market prices using Oracles #1674

OrderBook: Add market prices using Oracles #1674

Conversation

lemunozm commented Jan 5, 2024 • edited Loading

Description

Change list

lemunozm commented Jan 5, 2024

lemunozm left a comment

Choose a reason for hiding this comment

wischli left a comment

Choose a reason for hiding this comment

mustermeiszer left a comment

Choose a reason for hiding this comment

lemunozm commented Jan 10, 2024 • edited Loading

mustermeiszer left a comment

Choose a reason for hiding this comment

lemunozm commented Feb 1, 2024 • edited Loading

mustermeiszer left a comment

Choose a reason for hiding this comment

mustermeiszer Feb 2, 2024

Choose a reason for hiding this comment

lemunozm Feb 2, 2024

Choose a reason for hiding this comment

mustermeiszer Feb 2, 2024

Choose a reason for hiding this comment

lemunozm Feb 2, 2024

Choose a reason for hiding this comment

lemunozm Feb 2, 2024

Choose a reason for hiding this comment

wischli left a comment

Choose a reason for hiding this comment

mustermeiszer left a comment

Choose a reason for hiding this comment

lemunozm commented Feb 2, 2024

lemunozm commented Jan 5, 2024 •

edited

Loading

lemunozm commented Jan 10, 2024 •

edited

Loading

lemunozm commented Feb 1, 2024 •

edited

Loading